test: 핵심 도메인 서비스 단위 테스트 반영 + IDOR 3건 포함 버그 수정 (prod) - #220
Merged
Conversation
- JwtServiceTest: 토큰 발급/검증/만료/클레임 추출 검증 - UserIdResolverTest: 토큰 누락/만료/무효, 방문자 모드, userId 파싱 및 MDC 반영 검증 - dev-ci.yml: `-x test` 제거 — 지금까지 테스트가 아예 실행 안 되고 있었음
createCourse/getCourseByUser/getPrivateCourseByUser/getCourseDetail/updateCourse/ deleteCourses 전체 메서드에 대해 정상 케이스 + 예외 케이스 + 경계값을 검증. 테스트 작성 중 실제 프로덕션 코드에서 3가지 의심되는 부분을 발견해 별도로 표시해둠: - createCourse: 출발지 주소가 3토큰 미만이면 DepartureConverter가 null을 반환하고 이후 NPE로 이어짐 (요청값 검증 부재) - updateCourse: courseId로만 조회하고 userId로 소유자 검증을 하지 않아, 다른 사람의 코스 제목도 수정 가능 (IDOR 의심) - getCourseDetail: RunnectUser가 equals/hashCode를 오버라이드하지 않아 isNowUser 판정이 참조 동일성에 의존함 (같은 id라도 인스턴스가 다르면 다른 사람으로 판정될 수 있음)
1. DepartureConverter: 출발지 주소가 3토큰 미만이면 null 대신 BadRequestException(VALIDATION_DEPARTURE_ADDRESS_EXCEPTION)을 던지도록 변경 (기존엔 CourseService에서 바로 NPE로 이어짐) 2. CourseService.updateCourse: findById → findByCourseIdAndUserId로 변경해 본인 소유 코스만 수정 가능하도록 수정 (IDOR 방지, deleteCourses와 동일 패턴) 3. RunnectUser: equals/hashCode를 id 기준으로 구현. 기존엔 참조 동일성에 의존해 같은 유저라도 인스턴스가 다르면(Course.isMatchedUser, RecordService, PublicCourseService 등에서) 다른 사람으로 오판정될 수 있었음 CourseServiceTest의 관련 3개 테스트를 수정된 동작에 맞게 갱신하고, RunnectUserTest를 새로 추가해 equals/hashCode 자체를 검증.
createRecord/getRecordByUser/updateRecord/deleteRecords 전체 메서드에 대해 정상 케이스 + 예외 케이스 + 경계값 검증 (18개). 테스트 작성 중 CourseService.updateCourse와 동일한 패턴의 버그 발견해 수정: - updateRecord: userId 파라미터를 받지만 소유권 검증을 하지 않아 다른 사람의 기록 제목도 수정 가능했음 (IDOR). deleteRecords는 이미 소유권을 검증하고 있어서(PermissionDeniedException), 동일 패턴으로 맞춰서 수정. ErrorStatus.PERMISSION_DENIED_RECORD_UPDATE_EXCEPTION 추가. getRecordByUser의 건강 데이터 조회 실패 시 전체 요청은 실패하지 않고 healthData만 null로 우아하게 처리되는 방어 로직도 별도로 검증함.
getPublicCourseTotalPageCount/getMarathonPublicCourse/searchPublicCourse/ recommendPublicCourse/getPublicCourseByUser/getPublicCourseDetail/ createPublicCourse/deletePublicCourses/updatePublicCourse 전체 메서드에 대해 정상 케이스 + 예외 케이스 + 경계값 검증 (38개). 테스트 작성 중 발견해서 함께 수정한 버그 4건: 1. PublicCourse에 equals/hashCode 부재 — RunnectUser와 동일한 참조비교 문제. scrap 목록과 publicCourse 목록을 서로 다른 쿼리로 가져와 비교하는 곳이 5곳(getMarathonPublicCourse, searchPublicCourse, recommendPublicCourse, getPublicCourseByUser, getPublicCourseDetail)이라 isScrap이 잘못 표시될 수 있었음. id 기준 equals/hashCode 추가로 일괄 해결. 2. getPublicCourseDetail: 삭제된 코스 체크 조건이 반대(`== null`)였고, 심지어 예외를 생성만 하고 throw를 안 해서 완전히 죽은 코드였음. 조건 반전 + throw 추가. 3. updatePublicCourse: userId를 받으면서 소유권 검증을 안 해 다른 사람의 공개 코스 제목/설명도 수정 가능했음 (IDOR). deletePublicCourses와 동일한 관리자 예외 패턴으로 소유권 검증 추가. ErrorStatus.PERMISSION_DENIED_PUBLIC_COURSE_UPDATE_EXCEPTION 추가. 4. recommendPublicCourse: sort 파라미터가 "scrap"/"date" 둘 다 아니면 Page 변수가 null로 남아 NPE. 이미 정의돼 있던 INVALID_SORT_PARAMETER_EXCEPTION을 실제로 사용하도록 수정.
getMyPage/updateUserNickname/getUserProfile/deleteUser 전체 메서드에 대해 정상 케이스 + 예외 케이스 + 경계값 검증 (17개). 테스트 작성 중 발견해서 수정한 버그: - updateUserNickname: 중복 닉네임 체크를 유저 조회보다 먼저, 그리고 본인의 현재 닉네임과 비교 없이 수행하고 있어서, 본인의 기존 닉네임을 그대로 다시 저장하려고 해도 "이미 존재하는 닉네임"으로 거부됐음. 유저 조회를 먼저 하고, 요청 닉네임이 현재 닉네임과 다를 때만 중복 체크를 하도록 순서/조건 수정.
createAndDeleteScrap/getScrapCourseByUser 전체 메서드에 대해 정상 케이스 + 예외 케이스 + 경계값 검증 (9개). 테스트 작성 중 발견해서 수정한 버그: - createAndDeleteScrap: 스크랩한 적 없는 코스를 "취소"(scrapTF=false) 요청하면 scrap 변수가 null이라 scrap.updateScrapTF(false) 호출 시 바로 NPE(500)가 났음. 클라이언트가 중복 취소 요청을 보내거나 race condition만 있어도 쉽게 재현 가능한 케이스라 조건 분기 추가로 null이면 조용히 무시하도록 수정 (idempotent하게).
createHealthData/getHealthData/getHealthSummary/deleteHealthData 전체 메서드에 대해 정상 케이스 + 예외 케이스 + 경계값 검증 (22개). 버그는 발견되지 않음 — 소유권 검증(record.getRunnectUser().getId().equals(userId))을 일관되게 사용하고 있고, 동시성 경쟁으로 인한 유니크 제약 위반도 DataIntegrityViolationException을 잡아 409로 변환하는 등 이번에 테스트한 서비스 중 가장 방어적으로 잘 짜여있었음.
unam98
requested review from
RinRinPARK,
YuSuhwa-ve and
funnysunny08
as code owners
August 5, 2026 11:03
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
작업 배경
feat/banner-api-prod와 동일한 방식).MdcLoggingFilter,logback-spring.xml등 — dev 전용 모니터링 인프라 구축 작업에 딸려있음)에 의존하고 있어 함께 가져오지 않음.UserIdResolverTest의 MDC 관련 assertion 2건도 그래서 제거함.변경 사항
DepartureConverter,CourseService.updateCourseRunnectUserRecordService.updateRecordPublicCourse,PublicCourseServiceUserService.updateUserNicknameScrapService.createAndDeleteScrap영향 범위
updateCourse/updateRecord/updatePublicCourse세 곳 모두, userId를 받으면서도 소유권 검증을 안 해서 다른 사람의 리소스를 수정할 수 있었던 IDOR 취약점이었음. 이번 반영으로 소유자 본인만 수정 가능하도록 막힘.검증 매트릭스
소유자가_아니면_수정_불가소유자가_아니면_수정_불가소유자가_아님•
소유자가_아니면_수정_불가id가_같으면_인스턴스가_달라도_같다본인_현재_닉네임으로_재저장스크랩한_적_없는_코스_취소_요청은_무시된다Test Plan
./gradlew build전체(ServerApplicationTests 포함) 통과 확인docker run으로 임시 DB 띄워서 검증🤖 Generated with Claude Code